Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #77 and other race condition bugs #78

Merged
merged 3 commits into from
Dec 2, 2020

Conversation

conrad57
Copy link
Contributor

@conrad57 conrad57 commented Jul 4, 2020

Privacy - to indicate I read CONTRIBUTING.md.

This fixes the bugs described in #77. It furthermore fixes a similar bug: When adding a food it may not show in the overview list and the sum of total calories, until the overview activity is reloaded.
All these bugs happen, because the changes to the database are done via executor on another thread, so that the UI updates before the changes are written to the database.

@Kamuno Kamuno added the bug Something isn't working label Jul 12, 2020
Copy link
Member

@Kamuno Kamuno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. I haven't had the time to look over your other contributions. But a few comments regarding this one:

The purpose of LiveData is to update the data in the view without strong coupling and callbacks like this. Reinitiating the viewmodel manually every time the data changes seems like a very difficult solution to me.

It would be much easier to just change the MutableLiveData to MediatorLiveData and introduce a new getter in the DAO that returns LiveData. This way the LiveData will automatically be triggered when the database changes and thus the view will automatically update on database changes.

@conrad57
Copy link
Contributor Author

Thanks for the feedback.
I changed it to use LIveData now, which indeed looks like the better approach.

@conrad57 conrad57 requested a review from Kamuno July 16, 2020 12:12
@Kamuno Kamuno merged commit 6cd0855 into SecUSo:master Dec 2, 2020
@conrad57 conrad57 deleted the fix/master/really-bug-free branch April 20, 2021 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants